Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 11 clang-tidy checks #293

Merged
merged 18 commits into from
Jun 22, 2023
Merged

Conversation

bcaddy
Copy link
Collaborator

@bcaddy bcaddy commented Apr 28, 2023

Clang-Tidy Checks

Fixed and enabled the following clang-tidy Checks

  • performance-unnecessary-value-param check
  • performance-inefficient-vector-operation check
  • modernize-use-auto and its alias hicpp-use-auto.
  • performance-for-range-copy check
  • performance-faster-string-find check
  • modernize-use-default-member-init check
  • modernize-loop-convert tidy check
  • readability-const-return-type
  • readability-simplify-boolean-expr
  • google-build-namespaces (and its alias cert-dcl59-cpp)

Disabled cert-err58-cpp. It provides minimal benifit to us and flags every single gtest TEST macro as failing.

Other

  • Removed unsafe pointer alias to a DeviceVector pointer in io.cpp
  • Append make type to clang-tidy log files

@bcaddy bcaddy requested a review from ojwg April 28, 2023 20:59
@alwinm
Copy link
Collaborator

alwinm commented Apr 28, 2023

It looks good to me

@bcaddy bcaddy requested review from alwinm and removed request for ojwg April 28, 2023 21:05
Copy link
Collaborator

@alwinm alwinm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@bcaddy bcaddy changed the title Fix 7 clang-tidy checks Fix 12 clang-tidy checks May 15, 2023
@bcaddy bcaddy changed the title Fix 12 clang-tidy checks Fix 11 clang-tidy checks May 15, 2023
@bcaddy bcaddy force-pushed the dev-hackday-4-28 branch 4 times, most recently from 9349186 to 50415a7 Compare May 16, 2023 20:04
@@ -151,7 +151,7 @@ int Grid3D::Check_Custom_Boundary(int *flags, struct parameters P)
}

for (int i = 0; i < 6; i++) {
if (!((flags[i] >= 0) && (flags[i] <= 5))) {
if (flags[i] < 1 or flags[i] > 5) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know if these "or" and "not" statements are portable for other compilers?

Copy link
Collaborator Author

@bcaddy bcaddy Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They've been part of the C++ standard since C++98 so any standard compliant compiler will support them; see section 2.5 of the C++98 Standard.

MSVC is historically non-compliant with whatever standards they decide they don't like and so some versions from the mid 2010's and earlier don't support these alternative tokens directly but they can be enabled by either setting the right compiler flags or including the C compliance header.

@evaneschneider evaneschneider merged commit 82a6029 into cholla-hydro:dev Jun 22, 2023
@bcaddy bcaddy deleted the dev-hackday-4-28 branch June 29, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants